Feature: diag_manager by fmalatino · Pull Request #10 · fmalatino/pyFMS_old

fmalatino · 2025-03-10T14:34:28Z

Description
This PR brings in the currently available diag_manager modules in cFMS and removes the use of set_multipointer.

How Has This Been Tested?
Current CI is awaiting update to container.

Checklist:

My code follows the style guidelines of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
Any dependent changes have been merged and published in downstream modules
New check tests, if applicable, are included

mlee03 · 2025-03-28T17:17:14Z

+from .py_diag_manager.pyfms_diag_manager import pyFMS_diag_manager
 from .py_field_manager.py_field_manager import FieldTable
-from .py_horiz_interp.py_horiz_interp import HorizInterp
+from .py_horiz_interp.py_horiz_interp import pyFMS_horiz_interp


Why was this changed?

I changed it to match the names of the other modules. If desired it can be changed back.

let's change it back, the other modules need to be changed

mlee03 · 2025-03-28T17:18:20Z

+        self,
+        diag_model_subset: int = None,
+        time_init: NDArray = None,
+        err_msg: str = None,


err_msg is an out, it doesn't need to be a function argument

mlee03 · 2025-03-28T17:18:31Z

+    def diag_send_complete(
+        self,
+        diag_field_id: int,
+        err_msg: str = None,


likewise with err_msg

mlee03 · 2025-03-28T17:18:58Z

+        _cfms_diag_send_complete(diag_field_id_c, err_msg_c)
+
+        if err_msg is not None:
+            return err_msg_c.value.decode("utf-8")


don't need if else here?

mlee03 · 2025-03-28T17:19:08Z

+        minute: int,
+        second: int = None,
+        tick: int = None,
+        err_msg: str = None,


err_msg here too

mlee03 · 2025-03-28T17:28:29Z

+    diag axis init z
+    """
+
+    z = np.empty(shape=NZ, dtype=np.float64, order="C")


likewise with np.arange

mlee03 · 2025-03-28T17:30:21Z

+        field=var3,
+    )
+
+    for itime in range(24):


this was overlooked in cFMS, ntime=24 should be defined and the loop should become for itime in range(ntime)

mlee03 · 2025-03-28T17:31:33Z

+
+        ij = 0
+        for i in range(NX):
+            for j in range(NY):


these loops are not needed, they can be var2 = - var2?

mlee03 · 2025-03-28T17:32:21Z

+        diag_manager.diag_send_complete(diag_field_id=id_var2)
+
+    diag_manager.diag_end()
+


this was also overlooked in cFMS but the test should in the future have a check to read in the netcdf files and check values

mlee03 · 2025-03-28T17:33:19Z

        mpp.set_current_pelist(coarse_pelist)
        name = "test coarse domain"
-        maskmap = np.full(shape=(2, 4), fill_value=True, dtype=np.bool_, order="C")
+        maskmap = np.full(shape=8, fill_value=True, order="C")


was this change needed? cFMS should work with multidimensional numpy arrays

mlee03 · 2025-03-28T18:38:23Z

+from .py_diag_manager.pyfms_diag_manager import pyFMS_diag_manager
 from .py_field_manager.py_field_manager import FieldTable
-from .py_horiz_interp.py_horiz_interp import HorizInterp
+from .py_horiz_interp.py_horiz_interp import pyFMS_horiz_interp


let's change it back, the other modules need to be changed

mlee03 · 2025-03-31T11:48:50Z

+
+class DiagManager:
+
+    DIAG_ALL = 2


this should be retrieved from cfms

I have raised an issue for future handling of this; Issue 28

mlee03 · 2025-03-31T11:52:00Z

+        multiple_send_data: bool = None,
+    ) -> int:
+
+        module_name = module_name[:64]


module_name[:NAME_LENGTH]

This would also need to be amended post closing of Issue 28

mlee03 · 2025-03-31T11:53:04Z

+        realm_c, realm_t = set_Cchar(realm)
+        multiple_send_data_c, multiple_send_data_t = setscalar_Cbool(multiple_send_data)
+
+        if range.dtype == np.int32:


range is an optional argument, might not be the best argument to check for types

mlee03 · 2025-03-31T11:56:08Z

+    NZ = 2
+
+    domain_id = 0
+    calendar_type = 4


calendar related parameters should be imported from cfms

Also related to Issue 28

mlee03 · 2025-03-31T11:58:31Z

+    x = np.arange(NX, dtype=np.float64)
+
+    for i in range(NX):
+        x[i] = i


np.arange handles this. this for loop is not needed.

mlee03 · 2025-03-31T11:58:44Z

+    y = np.arange(NY, dtype=np.float64)
+
+    for j in range(NY):
+        y[j] = j


likewise, this for loop is not needed

mlee03 · 2025-03-31T11:58:56Z

+
+    z = np.arange(NZ, dtype=np.float64)
+
+    for k in range(NZ):


this for loop as well.

mlee03 · 2025-03-31T12:03:35Z

+    register diag field var3
+    """
+
+    axes_3d = np.array([id_x, id_y, id_z, 0, 0], dtype=np.int32)


axes array in cfms was declared to have 5 elements t avoid having 2d-5d flavors of the same subroutine. There is no need for this to be true in python. Users should be able to use axes_3d = [id_x, id_y, id_z] and the conversion to have five elements be handled inside the python function

mlee03 · 2025-03-31T12:04:44Z

+        for i in range(NX):
+            for j in range(NY):
+                for k in range(NZ):
+                    var3[ijk] = -1.0 * var3[ijk]


is there a short hand notation to replace this expanded set of for loops? This explicitness is required in C but not in python?

mlee03 · 2025-03-31T12:22:09Z

+    var3 = np.empty(shape=NX * NY * NZ, dtype=np.float32)
+
+    ijk = 0
+    for i in range(NX):


from python, the variables can be declared as multidimensional

Feature: diag_manager

fmalatino added 2 commits March 5, 2025 11:28

Added pyfms_diag_manager

cb51be4

Added public level diag_manager methods as of 14:27 6 March 2025

dea232f

fmalatino requested a review from mlee03 March 10, 2025 14:34

fmalatino added 11 commits March 12, 2025 10:59

Added in most recent additions to cFMS and cFMS/c_diag_manager

452151f

Merge branch 'main' into feature/diag_manager

fa2fd1c

Adding latest additions to diag_manager methods to match PR 10 in cFMS

1df9333

Adding pre-lim version of diag_manager test

e90c147

Linting

f250c11

Updated diag_manager methods to match cFMS

87d98cb

Switching to main

52a1ca5

Merge branch 'main' into feature/diag_manager

caec150

merge main after PR 14

77577bb

Merge branch 'main' into feature/diag_manager

6e5b7ba

Removed multipointer, updated files using it, linting

816b6da

fmalatino linked an issue Mar 26, 2025 that may be closed by this pull request

Remove set_multipointer #12

Closed

fmalatino added 2 commits March 27, 2025 11:23

Fixes for domains methods, getset and define, and their tests

1815571

linting

7144551

fmalatino marked this pull request as ready for review March 27, 2025 16:54

fmalatino added 6 commits March 27, 2025 15:23

Non-functioning version of diag_manager test

11f6e4b

Merge branch 'main' into feature/diag_manager

c74364b

Selecting for dimension and datatype in diag_manager

0faebd8

linting

a6ace68

Switching to main

676ccf7

Merge branch 'main' into feature/diag_manager

925f732

fmalatino marked this pull request as draft March 28, 2025 16:22

fmalatino added 4 commits March 28, 2025 12:23

Switching to main

2c692c4

Merge branch 'main' into feature/diag_manager

3470688

Updating testing methods

67d06f2

Changing the name of pyFMS_horiz_interp

b45703d

fmalatino marked this pull request as ready for review March 28, 2025 17:14

Merge branch 'feature/diag_manager' into feature/horiz_interp

239febb

mlee03 requested changes Mar 28, 2025

View reviewed changes

fmalatino added 4 commits March 28, 2025 13:57

Switching to feature/diag_manager

a8e2a67

Addressing PR comments

c880ca5

Pushed to wrong branch

3f5ae09

Addressing missed comments

e6b88ed

fmalatino requested a review from mlee03 March 28, 2025 19:43

mlee03 reviewed Mar 31, 2025

View reviewed changes

fmalatino added 5 commits March 31, 2025 16:06

Using multidimensional array in test_diag_manager

ae81148

Removing optional err_msg out from diag_manager functions

5653003

register_field_array axes as list now with appended 0's

ee744ea

Linting

dcd3494

More linting

d28da30

fmalatino requested review from bensonr and mlee03 April 2, 2025 16:56

mlee03 approved these changes Apr 10, 2025

View reviewed changes

fmalatino merged commit 821baa3 into main Apr 10, 2025
1 check passed

mlee03 pushed a commit to mlee03/pyFMS_old that referenced this pull request Apr 21, 2025

Merge pull request fmalatino#10 from fmalatino/feature/diag_manager

f6555f0

Feature: diag_manager

		diag_manager.diag_send_complete(diag_field_id=id_var2)

		diag_manager.diag_end()

Conversation

fmalatino commented Mar 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

fmalatino commented Mar 10, 2025 •

edited

Loading